[DwarfDebug] Avoid generating extra DW_TAG_subprogram entries #154636
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This test was added in 4d358b5 to fix the issue with emission of empty DW_TAG_subprogram tags (https://bugs.llvm.org/show_bug.cgi?id=12831). However, the test output is not checked properly, and it contains: ``` 0x00000206: DW_TAG_subprogram 0x00000207: DW_TAG_reference_type DW_AT_type (0x00000169 "class ") ``` The reason is that the DIE for the definition DISubprogram "writeExpr" is created during the call to `getOrCreateSubprogramDIE(declaration of writeExpr)`. Therefore, when `getOrCreateSubprogramDIE(definition of writeExpr)` is first called, we get a recursive chain of calls: ``` getOrCreateSubprogramDIE(definition of writeExpr) ... getOrCreateSubprogramDIE(declaration of writeExpr) ... getOrCreateSubprogramDIE(definition of writeExpr) ``` The outer call doesn't expect that the DIE for the definition of writeExpr will be created during the creation of declaration DIE. In this PR, a check is added to fix that. I'm not sure whether the problem described in https://bugs.llvm.org/show_bug.cgi?id=12831 is still relevant (I wasn't able to reproduce the problem with C++ reproducer from there). But if this test stays in repo, it should be fixed.
dzhidzhoev
requested review from
Michael137,
SLTozer,
adrian-prantl,
dwblaikie,
jmorse and
mstorsjo
|
@llvm/pr-subscribers-debuginfo Author: Vladislav Dzhidzhoev (dzhidzhoev) ChangesThis test was added in 4d358b5 to fix the issue with emission of empty DW_TAG_subprogram tags (https://bugs.llvm.org/show_bug.cgi?id=12831). However, the test output is not checked properly, and it contains: The reason is that the DIE for the definition DISubprogram "writeExpr" is created during the call to The outer call doesn't expect that the DIE for the definition of writeExpr will be created during the creation of declaration DIE. In this PR, a check is added to fix that. I'm not sure whether the problem described in https://bugs.llvm.org/show_bug.cgi?id=12831 is still relevant (I wasn't able to reproduce the problem with C++ reproducer from there). But if this test stays in repo, it should be fixed. Full diff: https://github.com/llvm/llvm-project/pull/154636.diff 2 Files Affected:
diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp b/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
index b03fac2d22a52..a99e63672b4bb 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
@@ -1351,6 +1351,10 @@ DIE *DwarfUnit::getOrCreateSubprogramDIE(const DISubprogram *SP, bool Minimal) {
ContextDIE = &getUnitDie();
// Build the decl now to ensure it precedes the definition.
getOrCreateSubprogramDIE(SPDecl);
+ // Check whether the DIE for SP has already been created after the call
+ // above.
+ if (DIE *SPDie = getDIE(SP))
+ return SPDie;
}
}
diff --git a/llvm/test/DebugInfo/X86/pr12831.ll b/llvm/test/DebugInfo/X86/pr12831.ll
index 402bc9aa04505..c37c017a52c29 100644
--- a/llvm/test/DebugInfo/X86/pr12831.ll
+++ b/llvm/test/DebugInfo/X86/pr12831.ll
@@ -1,4 +1,39 @@
-; RUN: llc %s -mtriple=x86_64-unknown-linux-gnu -o /dev/null
+; RUN: llc %s -mtriple=x86_64-unknown-linux-gnu -filetype=obj -o - | llvm-dwarfdump - | FileCheck %s
+
+; Check that there are no empty DW_TAG_subprogram tags.
+; CHECK: DW_TAG_subprogram
+; CHECK-NEXT: DW_AT
+; CHECK: DW_TAG_subprogram
+; CHECK-NEXT: DW_AT
+; CHECK: DW_TAG_subprogram
+; CHECK-NEXT: DW_AT
+; CHECK: DW_TAG_subprogram
+; CHECK-NEXT: DW_AT
+; CHECK: DW_TAG_subprogram
+; CHECK-NEXT: DW_AT
+; CHECK: DW_TAG_subprogram
+; CHECK-NEXT: DW_AT
+; CHECK: DW_TAG_subprogram
+; CHECK-NEXT: DW_AT
+; CHECK: DW_TAG_subprogram
+; CHECK-NEXT: DW_AT
+; CHECK: DW_TAG_subprogram
+; CHECK-NEXT: DW_AT
+; CHECK: DW_TAG_subprogram
+; CHECK-NEXT: DW_AT
+; CHECK: DW_TAG_subprogram
+; CHECK-NEXT: DW_AT
+; CHECK: DW_TAG_subprogram
+; CHECK-NEXT: DW_AT
+; CHECK: DW_TAG_subprogram
+; CHECK-NEXT: DW_AT
+; CHECK: DW_TAG_subprogram
+; CHECK-NEXT: DW_AT
+; CHECK: DW_TAG_subprogram
+; CHECK-NEXT: DW_AT
+; CHECK: DW_TAG_subprogram
+; CHECK-NEXT: DW_AT
+; CHECK-NOT: DW_TAG_subprogram
target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"
|
|
Can you rename the PR/commit title to state what is being fixed. I.e., something like "avoid generating empty DW_TAG_subprogram entries". Also, could you elaborate on how |
dzhidzhoev
changed the title
dzhidzhoev
changed the title
Sure!
It happens during the creation of a global (static local) variable "__stored_locally" in It's not completely clear to me, why we get this particular chain of calls, as the original reproducer for this test is quite confusing. It may be the case that this test is not relevant for Clang-produced code anymore. I've stumbled upon a probem with this test when trying to make a patch allowing two Functions to refer to the same DISubprogram (which was suggested here #142166 (comment)). I've added an assert that ensures that DIEs for the same DISubprogams are not created twice, and such assert gets triggered by this test. https://reviews.llvm.org/D144007 could also solve the problem (as subprogram DIEs would be instantiated before global variable creation), but I'm not sure it can be merged before https://reviews.llvm.org/D144006 (the latest attempt of merging it was #119001). I can try reverting this fix after https://reviews.llvm.org/D144007 gets merged. #142166 could also solve the problem, as it would break the link between !26 and !5 (!DICompositeType would be placed directly inside the declaration subprogram), but there is an argument against this patch #142166 (comment). Here's the stacktrace showing the recursive call to |
jmorse
approved these changes
Aug 27, 2025
There was a problem hiding this comment.
This makes sense / I think I understand it, we can end up committing to creating a DIE in getOrCreateSubprogramDIE, but then create ourselves between committing and actually creating the DIE. I've seen these random-nameless-entryless DW_TAG_subprograms in many files, fixing this is most welcome.
IMO the comment added to DwarfUnit.cpp should signal that it's a workaround, to encourage someone in the future to either fix it, or not get in their way if they're fixing it. Ideally the test would be more focused, but it's pre-existing, after all.
|
Thank you! @Michael137, please let me know if you have concerns. Otherwise, I'll merge it. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The test llvm/test/DebugInfo/X86/pr12831.ll was added in 4d358b5 to fix the issue with emission of empty DW_TAG_subprogram tags (https://bugs.llvm.org/show_bug.cgi?id=12831).
However, the test output is not checked properly, and it contains:
The reason is that the DIE for the definition DISubprogram "writeExpr" is created during the call to
getOrCreateSubprogramDIE(declaration of writeExpr). Therefore, whengetOrCreateSubprogramDIE(definition of writeExpr)is first called, we get a recursive chain of calls:The outer call doesn't expect that the DIE for the definition of writeExpr will be created during the creation of declaration DIE. So, another DIE is created for the same subprogram. In this PR, a check is added to fix that.
I'm not sure whether the problem described in https://bugs.llvm.org/show_bug.cgi?id=12831 is still relevant (I wasn't able to reproduce the problem with C++ reproducer from there). But if this test stays in repo, it should be fixed.